Skip to content

Conversation

@pcrespov
Copy link
Member

@pcrespov pcrespov commented Oct 6, 2025

What do these changes do?

This PR migrates database access from aiopg to asyncpg in the following components:

  • The payments domain within the webserver
  • The simcore_postgres_database.utils_payments module and its associated test suite

In detail summary (AI generated)

This pull request refactors the payments database utility and its usage across the codebase to fully adopt SQLAlchemy's async API, removing legacy aiopg types and error handling. It also updates test cases and web server modules to use the new async interfaces and corrects model naming for autorecharge logic. These changes modernize the codebase, improve compatibility with SQLAlchemy, and simplify transaction handling.

Database API modernization:

  • Replaced all usage of aiopg.sa.connection.SAConnection and related types with sqlalchemy.ext.asyncio.AsyncConnection and SQLAlchemy's async result types in utils_payments.py, updating function signatures and result handling accordingly. [1] [2] [3] [4] [5] [6]
  • Updated error handling in payment transaction functions to catch sqlalchemy.exc.IntegrityError instead of custom aiopg_errors.UniqueViolation.

Test suite updates:

  • Refactored all payment transaction tests to use AsyncEngine and async context management for database connections, replacing legacy connection types and adapting result assertions to new SQLAlchemy row objects. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]

Web server module updates:

  • Removed references to aiopg_errors.DatabaseError in exception handling for guest user and project owner removal logic, relying solely on asyncpg and domain-specific errors. [1] [2] [3]

Autorecharge model and API corrections:

  • Renamed PaymentsAutorechargeDB to PaymentsAutorechargeGetDB and updated all references and API conversion functions to use the new model name, ensuring consistency between DB and API layers. [1] [2] [3] [4] [5] [6]
  • Updated autorecharge DB module to use async engine and transaction context utilities for database operations.

These changes collectively improve code maintainability, test reliability, and compatibility with modern async database patterns.

Related issue/s

How to test

cd packages/postgres-database
make install-dev
pytest -vv tests/test_models_payments_transactions.py
cd services/web/server
make install-dev
pytest -vv tests/unit/**/wallets/**/test_*.py

Dev-ops

None

@pcrespov pcrespov added this to the Cheops milestone Oct 6, 2025
@pcrespov pcrespov self-assigned this Oct 6, 2025
@pcrespov pcrespov added a:webserver webserver's codebase. Assigning the area is particularly useful for bugs a:database associated to postgres service and postgres-database package labels Oct 6, 2025
@codecov
Copy link

codecov bot commented Oct 6, 2025

Codecov Report

❌ Patch coverage is 63.51351% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.97%. Comparing base (9d5b558) to head (156dc6d).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8462      +/-   ##
==========================================
+ Coverage   87.42%   87.97%   +0.54%     
==========================================
  Files        1912     1861      -51     
  Lines       74965    73182    -1783     
  Branches     1333     1063     -270     
==========================================
- Hits        65540    64380    -1160     
+ Misses       9026     8474     -552     
+ Partials      399      328      -71     
Flag Coverage Δ
integrationtests 64.14% <29.62%> (-0.01%) ⬇️
unittests 86.58% <63.51%> (+0.51%) ⬆️
Components Coverage Δ
pkg_aws_library ∅ <ø> (∅)
pkg_celery_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library 93.07% <ø> (ø)
pkg_notifications_library 85.20% <ø> (ø)
pkg_postgres_database 87.94% <90.90%> (-0.02%) ⬇️
pkg_service_integration ∅ <ø> (∅)
pkg_service_library 70.92% <0.00%> (ø)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 84.95% <ø> (ø)
agent 93.53% <ø> (ø)
api_server 91.78% <ø> (ø)
autoscaling 95.71% <ø> (ø)
catalog 92.36% <ø> (ø)
clusters_keeper 99.13% <ø> (ø)
dask_sidecar 92.35% <ø> (ø)
datcore_adapter 97.94% <ø> (ø)
director 75.81% <ø> (ø)
director_v2 90.93% <ø> (+0.01%) ⬆️
dynamic_scheduler 96.71% <ø> (∅)
dynamic_sidecar 90.43% <ø> (ø)
efs_guardian 89.74% <ø> (ø)
invitations 91.41% <ø> (ø)
payments 92.71% <50.00%> (ø)
resource_usage_tracker 92.20% <ø> (+0.16%) ⬆️
storage 86.74% <ø> (+0.29%) ⬆️
webclient ∅ <ø> (∅)
webserver 87.42% <64.81%> (-0.23%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9d5b558...156dc6d. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mergify
Copy link
Contributor

mergify bot commented Oct 6, 2025

🧪 CI Insights

Here's what we observed from your CI run for 156dc6d.

✅ Passed Jobs With Interesting Signals

Pipeline Job Signal Health on master Retries 🔍 CI Insights 📄 Logs
CI unit-tests Base branch is broken, but retries were needed. Could be early signs of flakiness 👀 Broken 1 View View

@pcrespov pcrespov force-pushed the mai/payments-to-asyncpg branch from b9a0f03 to 1efb2c3 Compare October 6, 2025 16:25
@pcrespov pcrespov changed the title WIP: Mai/payments to asyncpg ♻️ [Maintenance] Refactor Payments Domain to Use asyncpg Instead of aiopg Oct 6, 2025
@pcrespov pcrespov added the t:maintenance Some planned maintenance work label Oct 6, 2025
@pcrespov pcrespov force-pushed the mai/payments-to-asyncpg branch from 4eeccc2 to eccda81 Compare October 6, 2025 16:50
@pcrespov pcrespov marked this pull request as ready for review October 6, 2025 16:50
@pcrespov pcrespov added the 🤖-automerge marks PR as ready to be merged for Mergify label Oct 6, 2025
@pcrespov
Copy link
Member Author

pcrespov commented Oct 6, 2025

@mergify queue

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR migrates the payments domain from legacy aiopg to modern asyncpg database access patterns. The refactoring modernizes database interactions by adopting SQLAlchemy's async API throughout the payments system while maintaining backwards compatibility.

  • Replace aiopg.sa.connection.SAConnection with sqlalchemy.ext.asyncio.AsyncConnection across all payment modules
  • Update error handling from aiopg_errors to sqlalchemy.exc exceptions
  • Modernize database result handling using SQLAlchemy's async result methods

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
services/web/server/src/simcore_service_webserver/projects/models.py Remove aiopg imports and type aliases
services/web/server/src/simcore_service_webserver/payments/_onetime_db.py Migrate to asyncpg engine and async connection patterns
services/web/server/src/simcore_service_webserver/payments/_onetime_api.py Update database engine usage and model references
services/web/server/src/simcore_service_webserver/payments/_methods_db.py Comprehensive migration to async patterns with new connection handling
services/web/server/src/simcore_service_webserver/payments/_methods_api.py Update model type references
services/web/server/src/simcore_service_webserver/payments/_autorecharge_db.py Migrate autorecharge functionality to async patterns
services/web/server/src/simcore_service_webserver/payments/_autorecharge_api.py Update model references for consistency
services/web/server/src/simcore_service_webserver/garbage_collector/_core_utils.py Remove aiopg error handling
services/web/server/src/simcore_service_webserver/garbage_collector/_core_guests.py Remove aiopg error handling
services/payments/src/simcore_service_payments/db/auto_recharge_repo.py Update autorecharge statement class name
packages/service-library/src/servicelib/aiohttp/db_asyncpg_engine.py Modernize app key usage with proper typing
packages/postgres-database/tests/test_utils_payments_autorecharge.py Comprehensive test migration to async patterns
packages/postgres-database/tests/test_models_payments_transactions.py Update test patterns for async database operations
packages/postgres-database/src/simcore_postgres_database/utils_payments_autorecharge.py Rename class for consistency
packages/postgres-database/src/simcore_postgres_database/utils_payments.py Core migration from aiopg to asyncpg patterns

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@mergify
Copy link
Contributor

mergify bot commented Oct 6, 2025

queue

🟠 Waiting for conditions to match

  • -closed [📌 queue requirement]
  • -conflict [📌 queue requirement]
  • -draft [📌 queue requirement]
  • any of: [📌 queue -> configuration change requirements]
    • -mergify-configuration-changed
    • check-success = Configuration changed
  • any of: [🔀 queue conditions]
    • all of: [📌 queue conditions of queue default]
      • #approved-reviews-by >= 2 [🛡 GitHub branch protection]
      • #approved-reviews-by>=2
      • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
      • #changes-requested-reviews-by=0
      • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
      • #review-threads-unresolved=0
      • -conflict
      • -draft
      • base=master
      • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
      • label!=🤖-do-not-merge
      • label=🤖-automerge
      • any of: [🛡 GitHub branch protection]
        • check-skipped = deploy to dockerhub
        • check-neutral = deploy to dockerhub
        • check-success = deploy to dockerhub
      • any of: [🛡 GitHub branch protection]
        • check-success = system-tests
        • check-neutral = system-tests
        • check-skipped = system-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = unit-tests
        • check-neutral = unit-tests
        • check-skipped = unit-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = check OAS' are up to date
        • check-neutral = check OAS' are up to date
        • check-skipped = check OAS' are up to date
      • any of: [🛡 GitHub branch protection]
        • check-success = integration-tests
        • check-neutral = integration-tests
        • check-skipped = integration-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = build-test-images (frontend) / build-test-images
        • check-neutral = build-test-images (frontend) / build-test-images
        • check-skipped = build-test-images (frontend) / build-test-images
      • any of: [🛡 GitHub branch protection]
        • check-success = SonarCloud Code Analysis
        • check-neutral = SonarCloud Code Analysis
        • check-skipped = SonarCloud Code Analysis

@pcrespov pcrespov enabled auto-merge (squash) October 6, 2025 17:05
Copy link
Collaborator

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this 🥇

Copy link
Contributor

@wvangeit wvangeit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 6, 2025

Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

@pcrespov pcrespov merged commit 313b982 into ITISFoundation:master Oct 7, 2025
144 of 148 checks passed
@pcrespov pcrespov deleted the mai/payments-to-asyncpg branch October 7, 2025 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖-automerge marks PR as ready to be merged for Mergify a:database associated to postgres service and postgres-database package a:webserver webserver's codebase. Assigning the area is particularly useful for bugs t:maintenance Some planned maintenance work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants